Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmake: fix problem that go_library is never rebuilt. #2954

Merged
merged 1 commit into from
Jul 19, 2017

Conversation

helinwang
Copy link
Contributor

@helinwang helinwang commented Jul 19, 2017

merge_static_libs also have the similar logic of using ${dummyfile},
I am not sure if there needs a change or not.

`merge_static_libs` also have the similar logic of using ${dummyfile},
I am not sure if there needs a change or not.
@helinwang helinwang requested a review from gangliao July 19, 2017 01:07

# This custom command will always run since it depends on a not
# existing file.
add_custom_command(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Naive question: why add a custom command since we already have file(WRITE ${dummyfile} "const char * dummy = \"${dummyfile}\";") below?

Copy link
Contributor Author

@helinwang helinwang Jul 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my understanding file will create a file when cmake is called, during the Makefile generation process. But it will never be called when make is called. This has the problem that after the first time building, go_library never gets build again even the source code changed.

In contrast, the custom command is called when make is called, forcing the rebuild.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. Thanks! No rebuild is due to we build go libs by add_custom_command , so make will not track the source file changes.

Copy link
Contributor

@typhoonzero typhoonzero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!


# This custom command will always run since it depends on a not
# existing file.
add_custom_command(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. Thanks! No rebuild is due to we build go libs by add_custom_command , so make will not track the source file changes.

# existing file.
add_custom_command(
OUTPUT dummy_rebulid_${TARGET_NAME}
COMMAND cmake -E touch ${dummyfile}
Copy link
Contributor

@gangliao gangliao Jul 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that being built every time? Even if there no changes?

Copy link
Contributor Author

@helinwang helinwang Jul 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I thought it's better to rebuild even there is no change than not rebuild when there is change.

It's hard to track if the library source to be built changed or not, since the files may depend on other files that have changed located in other directories.

The good news is Go compiler caches the build result for all dependencies when they have not changed.

@helinwang helinwang merged commit 7a0e772 into PaddlePaddle:develop Jul 19, 2017
@helinwang helinwang deleted the cmake_rebuild branch July 19, 2017 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants